@mishantrop - First of all I'd like to express that I appreciate you taking the time to review this PR. It would, however, be much more useful for you to review the functionality and the usability of what is being implemented rather than getting into the weeds with super fine-grained code suggestions and what are, in some instances, personal coding/code-style preferences.

Does the PR solve the problem it's trying to solve? Does it do it in a way that improves the UI? Does it bring a useful new feature? I'm certainly not looking to ignore your suggestions, and if I and others feel some of them would provide an improvement I'll make the change(s). I'm just saying I think your initial focus is in the wrong place.

@smg6511 - First of all I'd like to express that I appreciate you taking the time to write new features..

> It would, however, be much more useful for you to review the functionality and the usability of what is being implemented
Changes to the code can affect functionality. Therefore, first of all, I pay attention to the quality (maintainability, readability, structure etc) of the code.

> in some instances, personal coding/code-style preferences
In "some", but not all. Updated my nit-comments.

> Does the PR solve the problem it's trying to solve?
I do not question the value of your PR. But I want to take part in the development of MODX and it is important for me that the code is pleasant to work with.

> I'm just saying I think your initial focus is in the wrong place.
Don't take my comments personally, this is a common practice in code reviews. Reviewers writes comments, and the author fixes the code or does not fix it. The main thing is to find a compromise.

@smg6511 You're on fire! There's a lot to cover in this PR. Please take a look at the technical implementation of PR #14009. Some similar changes have been made relevant to some of yours.

I'll do my best and find some time in the upcoming days to provide feedback on the functionality and usability of the fixes and new features in this PR.

@JoshuaLuckers - I appreciate the encouragement! Yes, this initial PR is a bit to review, I know ;-) But, once finalized, it will provide an pretty easy framework to update the rest of the grids. I'll take a look at 14009 in just a bit...

OK, I just took a quick look through 14009 and I don't see any conflicts with what I've done. Even so, assuming that my PR in some form gets merged and I move on to other grids (in 14009's case the policy/policy template grids), I'll refer back to 14009 to ensure I don't walk back anything important.

@mishantrop - Don't worry, I'm not taking anything personally. And I, of course, agree that code quality, consistency, and conformity to a standard is also very important. (I also know from comments on other issues/PRs that you and I might not share the same viewpoint on some stylistic and implementation conventions, but that's Ok.) I was just pointing out that I don't think that's the first thing to focus on when you're reviewing a PR.

@JoshuaLuckers - Don't know if you withdrew the comments you were making re #14009. (I was about to respond and they disappeared.) Anyway, I see I'll want to carry through how the core items are defined in constants in each object's base class, as they were done in the modAccessPolicy one.

However, I do think my implementation of the final relay of those permissions via prepareRow of each object class is more straightforward and immediately readable. So when you're reading the record in js, your getting, for example:

id: 3,
permissions: {
   create: false,
   update: true,
   delete: false
}

instead of:

id: 3,
cls: 'pedit'

The way I've done it is also very easy to iterate through without any additional fuss.

Note also, I think it makes sense is to use CRUD nomenclature across the back end (in code), which is why I've changed for example edit to update and remove to delete. On the front end, it makes more sense (in my mind at least) to use natural language (e.g., Edit X instead of Update X). Could be over-thinking it though ;-)

Yes I have deleted my comment. I have a lot to share but I still need to find time to properly work it out haha.

But your in the right direction with where I want to go, but you're right about the class names like pedit.

If your on Slack I could share my unstructured notes.

Yes, I am on Slack for another project I work on. How shall we connect?

@smg6511 can you please ping me once finishing with updates?

@theboxer Will do. The question now is: In addition to doing any last tweaks/optimizations to what is here now, shall I carry this through the rest of the grids as a part of this PR, or leave that to a supplementary PR? (I don't believe the changes made thus far would negatively affect grids that have not been updated to use the new methods.)

@smg6511 how many affected grid you think there are, that will need an update?

@theboxer Ultimately all editor grids need an update of some sort to ensure the gear icon/menu only shows when applicable. I'll survey which grids still need work—two that come to mind that I'm working on now are the Policy and Policy Templates grids.

@smg6511 I guess it'd be fine to have them all here, but I'm not sure how much of an effort it is, so it'll all get into 3.1.

@theboxer @opengeek - Guys, there's something I hadn't noticed initially about the additional "Creator" column I've added as a feature of this PR: Because the creator data point is not a part of the db data, I'm finding it difficult (probably impossible) to provide an expected secondary sort (on the name column) when sorting the "Creator" column. So, when sorting on "Creator," the name column is just randomly sorted (which I don't think is great). (I had a little hack going to build the Creator column on the fly, based on extra data generated from various GetList Processors.)

Anyway, upon finding the secondary sorting issue, I tried to pull another rabbit out of the hat, but I think this ultimately points to the need for one or two more columns in many of the db tables to achieve the desired grouping (and even better, filtering) of objects based on how they were created (i.e., by modx, by an Extra, or by a User). This is probably not a job for this PR. So, I'm thinking of removing that aspect for now.

But I would like to get a read on whether you'd go for making some schema changes to add a general creator column and perhaps a couple others[1] to many of the tables (e.g., namespaces, contexts, sources, roles, policies, policy templates, etc.) where "ownership" of the row data can be useful. We could also lose the hard-coded constants in the GetList Processors that indicate protected system-installed data. Right now, the way of identifying protected items (by name in the aforementioned constants) seems a little fragile.

[1] Other columns?

• createdby (specific User id when applicable)
• protected (boolean, whether the row can be deleted by User action [as opposed to a system process, such as modx installation/upgrade or an Extra being uninstalled])

Why wouldn't you just sort on the joined table columns for creator?

As it stands, creator doesn't exist in the db; it's derived from other "knowns" that are gathered on the php side of things (e.g., for Contexts, I know which are installed by modx via the hard-coded constants assessed by the isCoreContext method). So there's no way via the joins on the sql side (the trick used was sorting via the mysql FIELD() method, which is being fed an explicit list of names [presorted groupings of modx-installed names and, in the case of namespaces, extras-installed namespaces]). Yes, I could do something like create a temporary table to work with that arbitrary data as a "real" column ... but that'd be crazy and you'd have to demote me to a new repo status of "observer only" ;-) .

As it stands, creator doesn't exist in the db; it's derived from other "knowns" that are gathered on the php side of things (e.g., for Contexts, I know which are installed by modx via the hard-coded constants assessed by the isCoreContext method). So there's no way via the joins on the sql side (the trick used was sorting via the mysql FIELD() method, which is being fed an explicit list of names [presorted groupings of modx-installed names and, in the case of namespaces, extras-installed namespaces]). Yes, I could do something like create a temporary table to work with that arbitrary data as a "real" column ... but that'd be crazy and you'd have to demote me to a new repo status of "observer only" ;-) .

Isn't creator just the name fields from user profile? I'm confused. It should absolutely be a simple join to that table from the createdby field.

I'm guessing you're thinking more in the context of Resources, where the so-called creator (createdby) is persisted in the data itself. I believe Resources are the only place that happens. But what I'm working with here doesn't apply to Resources—which are of course always created by a specific User. The "creator" I've added is a more holistic identification (modx, extra, user [in general, not specific id]). In any case, none of the other MODX objects save an identifier in the db as to who or what process created it.

@smg6511 How about you just disable the sort on the creator column? We can discuss different approaches later on, that could potentially replace the content of the column as defined in this PR and introduce the sorting/filtering in it as well.

How about you just disable the sort on the creator column?

Yeah, I'll go ahead and do that and remove the sorting-related php. Will ping you when ready...